Skip to content

incorrect_length_validation: Consider inclusion validations as an alternative#220

Merged
gregnavis merged 1 commit into
gregnavis:masterfrom
fatkodima:consider-inclusion-validations
Aug 9, 2025
Merged

incorrect_length_validation: Consider inclusion validations as an alternative#220
gregnavis merged 1 commit into
gregnavis:masterfrom
fatkodima:consider-inclusion-validations

Conversation

@fatkodima

Copy link
Copy Markdown
Contributor

Fixes #205.

@gregnavis gregnavis left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix! I left some comments.

Comment thread lib/active_record_doctor/detectors/incorrect_length_validation.rb Outdated
Comment thread lib/active_record_doctor/detectors/incorrect_length_validation.rb Outdated
Comment thread test/active_record_doctor/detectors/incorrect_length_validation_test.rb Outdated
Comment thread test/active_record_doctor/detectors/incorrect_length_validation_test.rb Outdated
Comment thread lib/active_record_doctor/detectors/incorrect_length_validation.rb Outdated
@fatkodima fatkodima force-pushed the consider-inclusion-validations branch from e88a478 to 7867d40 Compare April 24, 2025 11:52
@fatkodima

Copy link
Copy Markdown
Contributor Author

Made updates.

@gregnavis gregnavis removed the waiting label Aug 2, 2025

@gregnavis gregnavis left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fatkodima, I reviewed the PR again, and found two other edge cases to cover. We're almost at the finish line and I'd love to see this merged. Thank you for your work.

Comment thread test/active_record_doctor/detectors/incorrect_length_validation_test.rb Outdated
Comment thread lib/active_record_doctor/detectors/incorrect_length_validation.rb
@gregnavis gregnavis added the false positive An error is reported when it shouldn't label Aug 5, 2025
@gregnavis

Copy link
Copy Markdown
Owner

Also, could we add a test case with an enum?

@fatkodima fatkodima force-pushed the consider-inclusion-validations branch from 7867d40 to 4dd13fd Compare August 5, 2025 19:37
@fatkodima

Copy link
Copy Markdown
Contributor Author

Also, could we add a test case with an enum?

Needs changes to the checker and test cases. I think this is not really related to this PR and better done separately.

@fatkodima fatkodima force-pushed the consider-inclusion-validations branch from 4dd13fd to 4fbb40d Compare August 5, 2025 19:39
@gregnavis gregnavis merged commit 96b3dd6 into gregnavis:master Aug 9, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

false positive An error is reported when it shouldn't

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rails 7.1+ Enum Validation

2 participants